Skip to content

[http-client-csharp] Add ArgumentExpression for ref/out argument support#10845

Merged
JoshLove-msft merged 10 commits into
microsoft:mainfrom
JonathanCrd:joncarde/argument-expression-8399
Jul 9, 2026
Merged

[http-client-csharp] Add ArgumentExpression for ref/out argument support#10845
JoshLove-msft merged 10 commits into
microsoft:mainfrom
JonathanCrd:joncarde/argument-expression-8399

Conversation

@JonathanCrd

@JonathanCrd JonathanCrd commented May 30, 2026

Copy link
Copy Markdown
Member

Summary

Add a new ArgumentExpression class that wraps a ValueExpression with optional ref/out modifiers, cleanly separating argument-passing semantics from variable reference semantics.

Closes #8399

Changes

  • New ArgumentExpression class — wraps a ValueExpression with IsRef/IsOut, Write() visitor support via Accept(), and Update()
  • Simplified VariableExpression — removed IsRef/IsOut properties; now a pure variable reference (with backward-compatible shims)
  • Simplified ParameterProvider — removed dual _asVariable/_asArgument caching pattern; added GetArgumentExpression()
  • Updated AsArgument() — now returns ValueExpression (an ArgumentExpression) instead of VariableExpression
  • Added VisitArgumentExpression to LibraryVisitor
  • Updated call sites: ClientProvider, MrwSerializationTypeDefinition, JsonPatchSnippets, ScmModelProvider
  • Added tests: ArgumentExpressionTests (8 tests covering ref, out, update, and ParameterProvider integration) and ExpressionVisitorTests

@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label May 30, 2026
@pkg-pr-new

pkg-pr-new Bot commented May 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10845

commit: 988f018

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@JonathanCrd JonathanCrd force-pushed the joncarde/argument-expression-8399 branch from c34da4a to 30e4276 Compare June 2, 2026 22:09
@azure-sdk-automation

azure-sdk-automation Bot commented Jun 2, 2026

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

@JonathanCrd JonathanCrd force-pushed the joncarde/argument-expression-8399 branch from 30e4276 to 56c20d4 Compare June 2, 2026 23:14
@JonathanCrd JonathanCrd self-assigned this Jun 16, 2026
JonathanCrd and others added 5 commits June 16, 2026 16:08
… support

Add a new ArgumentExpression class that wraps a ValueExpression with
optional ref/out modifiers, separating argument-passing semantics from
variable reference semantics.

Changes:
- Create ArgumentExpression to handle ref/out keywords in argument context
- Remove IsRef/IsOut from VariableExpression (now a pure variable reference)
- Simplify ParameterProvider by removing the dual _asVariable/_asArgument pattern
- Update AsArgument() to return ArgumentExpression wrapping the variable
- Update call sites: ClientProvider, MrwSerializationTypeDefinition,
  JsonPatchSnippets, ScmModelProvider
- Add VisitArgumentExpression to LibraryVisitor
- Add comprehensive ArgumentExpressionTests

Closes microsoft#8399

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore the legacy VariableExpression ref/out constructor and add regression tests for ArgumentExpression visitor handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The pnpm setup:min step in RegenCheck CI fails on Linux runners
where NodeTool resolves Node.js 24.x to v24.14.1, which does not
satisfy the which@7.0.0 engine requirement (^24.15.0). Temporarily
relax engine-strict during workspace setup to unblock CI while the
runner images are updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Mark VariableExpression.IsRef/IsOut and legacy constructor as
  [Obsolete] to guide callers toward ArgumentExpression
- Replace ByRef() with ArgumentExpression in MrwSerializationTypeDefinition
  for method argument semantics (ref return in ScmModelProvider is correct)
- Fix dead ternary in ExpressionVisitorTests assertion
- Keep redundant 2-param VariableExpression(type, name) constructor since
  it is part of the public API surface and used widely

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JonathanCrd JonathanCrd force-pushed the joncarde/argument-expression-8399 branch from e7255fb to 29c8aa5 Compare June 16, 2026 23:32
JonathanCrd and others added 2 commits June 16, 2026 16:57
@typespec/http-client-csharp is not in the chronus workspace.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JonathanCrd JonathanCrd changed the title [WIP] feat(http-client-csharp): add ArgumentExpression for ref/out argument support [http-client-csharp] Add ArgumentExpression for ref/out argument support Jun 17, 2026
@JonathanCrd JonathanCrd marked this pull request as ready for review June 17, 2026 07:04
- Remove obsolete IsRef/IsOut shims and legacy constructor from VariableExpression
- Update ByRef snippet to return an ArgumentExpression
- Add ValueExpression.AsArgument snippet for readable ref/out argument wrapping
- Use ByRef/AsArgument at call sites (ClientProvider, MrwSerializationTypeDefinition, JsonPatchSnippets)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@jorgerangel-msft jorgerangel-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…ty body ternary

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JoshLove-msft JoshLove-msft added this pull request to the merge queue Jul 9, 2026
Merged via the queue into microsoft:main with commit 348d6be Jul 9, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp eng

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add better support for ArgumentExpression vs VariableExpression

3 participants